Skip to content
This repository has been archived by the owner on Dec 4, 2023. It is now read-only.

fix: Pass skipStep on main branch #7

Merged
merged 2 commits into from
Jan 26, 2023
Merged

fix: Pass skipStep on main branch #7

merged 2 commits into from
Jan 26, 2023

Conversation

mydea
Copy link
Member

@mydea mydea commented Jan 19, 2023

Currently, when you define a skip_step, this will be ignored when the size limit runs on the main/master branch. This can lead to discrepancies between runs on master & on the PRs, plus results in additional time. For sentry-javascript, this means each time on the master branch we re-run yarn install & yarn build for the size check, even though these things have been run & cached before.

I am not sure if this was done on purpose here #4, but it appears to me as if that would not be necessary here.

While at it, I also fixed another issue I noticed: Previously, if there have been changes initially, then you update the PR and have no changes, the comment may not have been updated. Now, we always update the comment if there has been one before.

@mydea mydea self-assigned this Jan 19, 2023
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came across this before and also wondered why we ignored skipStep here...

From my PoV this is a good change but I might also be missing some context around the original change.

Copy link
Member

@billyvg billyvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable

@mydea mydea merged commit 6c57d92 into master Jan 26, 2023
@mydea mydea deleted the main-skip-step branch January 26, 2023 16:05
@mydea mydea restored the main-skip-step branch January 26, 2023 17:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants